Skip to content

Fix for JsonNumber output in cache dump #68

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

jvanderwee
Copy link

Related to apollographql/apollo-kotlin-adapters#28

I've no idea how to unit test this, so any pointers appreciated 🙏

@apollo-cla
Copy link

@jvanderwee: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@@ -290,7 +290,7 @@ private fun Any?.toExternal(): Any? {
is Int -> this
is Long -> this
is Double -> this
is JsonNumber -> this
is JsonNumber -> this.value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toExternal() may be used in other places. Can you implement JsonNumber.toString() instead? (here)

I wouldn't worry too much about unit tests here. You could do something like

assertEquals("12345", JsonNumber("12345").toString())

But the logic is simple enough, I don't think it requires a test.

@jvanderwee
Copy link
Author

ah yes, that makes much more sense! apollographql/apollo-kotlin#6273

@jvanderwee jvanderwee closed this Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants